Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] - fixing infinite look on host flyout panel due to lack of memoization #204999

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Dec 19, 2024

Summary

This PR fixes an issue raised recently where opening a preview panel for a host of user on top of a flyout already showing a host or user was getting the UI stuck into an infinite loop.

While we found a few ways to fix the issue - primarily adding memoization to the UI components within the HostPanel and UserPanel, the approach in this PR fixes the issue at a more root level.

Infinite loop behavior

Screen.Recording.2024-12-19.at.1.20.02.PM.mov

Issue fixed

Screen.Recording.2024-12-19.at.1.18.46.PM.mov

This fix should also potentially bring some performance improvement to all the components that are using the hook (we have a few).

@PhilippeOberti PhilippeOberti added backport release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting:Investigations Security Solution Investigations Team backport:version Backport to applied version labels v8.18.0 labels Dec 19, 2024
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner December 19, 2024 19:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 21.4MB 21.4MB +185.0B

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for getting a fix in on this!

@PhilippeOberti PhilippeOberti merged commit 96264d2 into elastic:main Dec 23, 2024
8 checks passed
@PhilippeOberti PhilippeOberti deleted the fix-infinite-loop branch December 23, 2024 15:55
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12469487537

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 23, 2024
…o lack of memoization (elastic#204999)

## Summary

This PR fixes [an
issue](elastic/security-team#11424) raised
recently where opening a preview panel for a host of user on top of a
flyout already showing a host or user was getting the UI stuck into an
infinite loop.

While we found a few ways to fix the issue - primarily adding
memoization to the UI components within the HostPanel and UserPanel, the
approach in this PR fixes the issue at a more root level.

Infinite loop behavior

https://github.com/user-attachments/assets/92cb60ad-7801-43ec-a247-8943e091b6a8

Issue fixed

https://github.com/user-attachments/assets/30b30b42-f32e-4c02-9407-9d0f671d7216

This fix should also potentially bring some performance improvement to
all the components that are using the hook (we have a few).

(cherry picked from commit 96264d2)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 23, 2024
… due to lack of memoization (#204999) (#205104)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] - fixing infinite look on host flyout panel due
to lack of memoization
(#204999)](#204999)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-23T15:55:30Z","message":"[Security
Solution] - fixing infinite look on host flyout panel due to lack of
memoization (#204999)\n\n## Summary\r\n\r\nThis PR fixes
[an\r\nissue](elastic/security-team#11424)
raised\r\nrecently where opening a preview panel for a host of user on
top of a\r\nflyout already showing a host or user was getting the UI
stuck into an\r\ninfinite loop.\r\n\r\nWhile we found a few ways to fix
the issue - primarily adding\r\nmemoization to the UI components within
the HostPanel and UserPanel, the\r\napproach in this PR fixes the issue
at a more root level.\r\n\r\nInfinite loop
behavior\r\n\r\n\r\nhttps://github.com/user-attachments/assets/92cb60ad-7801-43ec-a247-8943e091b6a8\r\n\r\nIssue
fixed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/30b30b42-f32e-4c02-9407-9d0f671d7216\r\n\r\nThis
fix should also potentially bring some performance improvement to\r\nall
the components that are using the hook (we have a
few).","sha":"96264d29237f0b7cfc8c651393f12d248f0d84e0","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","v9.0.0","Team:Threat
Hunting:Investigations","backport:version","v8.18.0"],"title":"[Security
Solution] - fixing infinite look on host flyout panel due to lack of
memoization","number":204999,"url":"https://github.com/elastic/kibana/pull/204999","mergeCommit":{"message":"[Security
Solution] - fixing infinite look on host flyout panel due to lack of
memoization (#204999)\n\n## Summary\r\n\r\nThis PR fixes
[an\r\nissue](elastic/security-team#11424)
raised\r\nrecently where opening a preview panel for a host of user on
top of a\r\nflyout already showing a host or user was getting the UI
stuck into an\r\ninfinite loop.\r\n\r\nWhile we found a few ways to fix
the issue - primarily adding\r\nmemoization to the UI components within
the HostPanel and UserPanel, the\r\napproach in this PR fixes the issue
at a more root level.\r\n\r\nInfinite loop
behavior\r\n\r\n\r\nhttps://github.com/user-attachments/assets/92cb60ad-7801-43ec-a247-8943e091b6a8\r\n\r\nIssue
fixed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/30b30b42-f32e-4c02-9407-9d0f671d7216\r\n\r\nThis
fix should also potentially bring some performance improvement to\r\nall
the components that are using the hook (we have a
few).","sha":"96264d29237f0b7cfc8c651393f12d248f0d84e0"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204999","number":204999,"mergeCommit":{"message":"[Security
Solution] - fixing infinite look on host flyout panel due to lack of
memoization (#204999)\n\n## Summary\r\n\r\nThis PR fixes
[an\r\nissue](elastic/security-team#11424)
raised\r\nrecently where opening a preview panel for a host of user on
top of a\r\nflyout already showing a host or user was getting the UI
stuck into an\r\ninfinite loop.\r\n\r\nWhile we found a few ways to fix
the issue - primarily adding\r\nmemoization to the UI components within
the HostPanel and UserPanel, the\r\napproach in this PR fixes the issue
at a more root level.\r\n\r\nInfinite loop
behavior\r\n\r\n\r\nhttps://github.com/user-attachments/assets/92cb60ad-7801-43ec-a247-8943e091b6a8\r\n\r\nIssue
fixed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/30b30b42-f32e-4c02-9407-9d0f671d7216\r\n\r\nThis
fix should also potentially bring some performance improvement to\r\nall
the components that are using the hook (we have a
few).","sha":"96264d29237f0b7cfc8c651393f12d248f0d84e0"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels backport release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants